-
-
Notifications
You must be signed in to change notification settings - Fork 4.2k
New text features #21331
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
New text features #21331
Conversation
* Removed `TextFont` and `TextColor` from the `Text`, `Text2d`, and `TextSpan` requires, replaced with `ComputedTextStyle`. * `update_text_styles` updates the `ComputedTextStyle`s each frame from the text entities nearest ancestors with `TextFont` or `TextColor` components.
…a single get_style method
Yes I agree mostly, Text2d is fine with the current system of unpropagated per entity text style components. I think though that having two very similar but different APIs tends to be super miserable for users. So I'd rather keep working to converge Text and Text2d, at least as far as it's possible. There's always some poor confused person on Discord who has been battling for hours trying to make |
…/bevy into bevy-text-new-features
It looks like your PR has been selected for a highlight in the next release blog post, but you didn't provide a release note. Please review the instructions for writing release notes, then expand or revise the content in the release notes directory to showcase your changes. |
Seconding @viridia here: I really like this work and the broad direction, but want to try and chunk this down as best as we can. Seeing the central vision here was really compelling and useful, and gives me the confidence to start merging little chunks of this in. |
mut font_atlas_sets: ResMut<FontAtlasSets>, | ||
max_fonts: ResMut<MaxFonts>, | ||
active_font_query: Query<(&ComputedTextStyle, &ComputedFontSize)>, | ||
) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should have unit tests for this algorithm.
/// Wrapper used to differentiate propagated text style compoonents | ||
#[derive(Debug, Component, Clone, PartialEq, Reflect)] | ||
#[reflect(Component, Clone, PartialEq)] | ||
pub struct InheritedTextStyle<S: Clone + PartialEq>(pub S); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we are using the word Style
inconsistently here, does it refer to a single styling property or several? Maybe calling this InheritedTextProperty
might be clearer.
Yeah I guess I've been a bit impatient and getting ahead of myself. The main thing we need I guess is a final decision on whether this |
OK I'm a bit confused; I didn't notice the removal of |
It's not much different structurally, just without |
This seems controversial. It seems like While
|
Lets say you are displaying a score using:
that is updated by a system that queries for a
the updater system also needs to be changed. |
I mean, I really, really didn't like the text sections as entities changes, but left my objections too late. It's imo more confusing and difficult to use, internally it's much more complicated, and performance is terrible compared to before. It only starts to make sense with cascading text styles. |
Oh sorry, actually it wasn't because of usability issues, I completely forgot why I wanted to remove |
Right now, my reaction to For the changes to As far as "text spans as entities" goes: people have lately been doing some really neat stuff that simply wasn't possible before. And although we still don't have mixed text + non-text (like embedded icons in paragraphs), the goal is to have that ability eventually, and it's hard to see a path for doing that if spans are not entities. |
Is there a way we can embed this distinction inside of |
Initially |
Objective
New text features:
Val::Rem
variant, allows setting lengths relative to the default text style's font size.Fixes #21210, #21207, #21208, #21175, #9525, #5471
Solution
TextFont
has been removed. There are five new text components in its place:FontFace
,FontSize
,FontSmoothing
,TextColor
andLineHeight
. These can be placed on any entity and the values are propagated down the tree and used to update another new component,ComputedTextStyle
that is required by allText
andText2d
entities.DefaultTextStyle
resource.ComputedTextStyle
's fields unset by propagation take the values fromDefaultTextStyle
.Text
no longer requiresNode
.ComputedFontSize
component, holds the computed size of the font, used by the systems that track unused font atlases.TextRoot
component, added automatically to root text entities, holds a list of all the entities making up the text block, updated on changes.FontSize
is an enum withPx
,Rem
,Vw
,Vh
,VMin
, andVMax
.TextSpan
's removed and text style propagation support. bsn! should should make it easier to create and update hierarachies hopefully as well.Testing
The
Text
example has been changed to demonstrate some text usingFontSize::Rem
.Needs lots of testing, only updated a fraction of the examples so far.
Text2d
is ~40% worse than main.Text
seems to be the same, or slightly improved over main.free_unused_font_atlases
walks all text entities every update atm, needs a better implementation.bevy_app::propagate
. It's not completely terrible but has way too many systems, and I'm not sure about things like theInheritableTextStyle
trait.Text
andText2d
entities still require components likeTextBlock
,TextLayoutInfo
, which just sit unused. Could switch to storing the generated layouts in a resource.